-
Notifications
You must be signed in to change notification settings - Fork 219
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Bump Optimization to v4, and related packages accordingly #2354
Conversation
Failing test MWE using Random
using Turing
import ReverseDiff
@model function f()
x ~ Normal(0, 1)
end
Random.seed!(222)
# cons_args is mandatory, if no constraints are passed it runs fine
cons(res, x, _) = (res .= [x[1]])
cons_args = (cons=cons, lcons=[0], ucons=[Inf])
initial_params = [0.5]
m1 = Turing.Optimisation.estimate_mode(
f(), MAP(); initial_params=initial_params, cons_args...
) Show the error traceback
Output of ]st --manifest
versioninfo()
|
a504d0f
to
3c91eec
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #2354 +/- ##
==========================================
- Coverage 86.39% 86.26% -0.13%
==========================================
Files 22 22
Lines 1573 1573
==========================================
- Hits 1359 1357 -2
- Misses 214 216 +2 ☔ View full report in Codecov by Sentry. |
Pull Request Test Coverage Report for Build 11073388637Details
💛 - Coveralls |
This method ambiguity is fixed in JuliaDiff/ForwardDiff.jl#687, it just needs a release |
I think bumping Optimization to v4 (which in turn bumps OptimizationBase to v2) would also allow it to work with Mooncake as an AD backend (since it now goes via DifferentiationInterface, SciML/OptimizationBase.jl#108). Both the MLE and MAP tests pass with Optimization@4 and adtype=AutoMooncake. |
Maybe mark ForwardDiff-related tests as broken and get this merged? |
@yebai can do! |
It's curious, but the ForwardDiff tests that were failing previously seem to have been fixed on Julia 1.11. These were the previous failures: https://github.com/TuringLang/Turing.jl/actions/runs/11072371830/job/30766467971 This was pre-1.11 so the CI was running on 1.10.5. In contrast, none of those tests fail here: https://github.com/TuringLang/Turing.jl/actions/runs/11483911558/job/31960475967 The only failure here is because Mooncake was marked as broken, which it isn't. The method that was ambiguous was |
The remaining test failure is #2371. I could rerun CI to get the green tick, but don't see much of a point. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good!
Feel free to merge. |
CI passed on the main branch after we merged 😄 🎉 |
Re-opening #2327.
Closes #2348
Closes #2349
Closes #2350
Closes #2351
Closes #2352
Closes #2353